[POS Settings] Store section and `PointOfSaleSettingsService` #16034

iamgabrielma · 2025-08-25T08:48:20Z

Description

This PR adds Store and POS receipt information to the Store section in POS Settings. UI is not handled yet, only fetching store and POS receipt details.

Testing information

On a store with Woo 10+ and some POS receipt settings setup, go to POS > ... > Settings. Observe these load upon tapping on the Store section:

Screen.Recording.2025-08-25.at.17.15.01.mov

Update PointOfSaleSettingsService.retrievePOSReceiptSettings WooCommerce version to an unsupported version, ie higher.

- shouldShowReceiptInformation = await isPluginSupported(.wooCommerce, minimumVersion: "10.0")
+ shouldShowReceiptInformation = await isPluginSupported(.wooCommerce, minimumVersion: "12.0")

Observe that only store details are loaded.

Simulator Screenshot - iPad mini (A17 Pro) - US store - 2025-08-25 at 17 18 44

Question/Feedback

I initially linked this service it to the aggregate model, following the same design as other services, but ultimately this felt a bit overkill at this point for the current state of settings we'll need (all of them in this PR, the rest of the project's i1 is navigation and details in the card reader service) so I went with the simplest way to initialize it within PointOfSaleSettingsView.

Would you suggest a different approach? Should we go through exposing the service through the aggregate model? Happy to handle either for this PR or a separate one.

wpmobilebot · 2025-08-25T08:57:25Z

📲 You can test the changes from this Pull Request in WooCommerce iOS Prototype by scanning the QR code below to install the corresponding build.

	App Name	WooCommerce iOS Prototype
	Build Number	`pr16034-cf78712`
	Version	`23.1`
	Bundle ID	`com.automattic.alpha.woocommerce`
	Commit	`cf78712`
	Installation URL	2p3aild8u6ffo

Automatticians: You can use our internal self-serve MC tool to give yourself access to those builds if needed.

This reverts commit 8a02151.

jaclync

Shared some initial comments, let me know what you think! Also, would be nice having unit tests.

WooCommerce/Classes/POS/Models/PointOfSaleSettingsService.swift

dangermattic · 2025-08-26T06:45:01Z

	1 Warning
⚠️	This PR is larger than 300 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

WooCommerce/Classes/POS/Models/PointOfSaleSettingsController.swift

iamgabrielma · 2025-08-26T11:11:43Z

Thanks for the review @jaclync , this is ready for another look. I split the initial service into PointOfSaleSettingsController and PointOfSaleSettingsService, with their respective protocols, previews, and tests. There's some improvements to be made, I left some comments, but is getting a bit bigger so I'll handle those separately.

jaclync

Thanks for the updates, works as expected some unblocking comments just for your consideration.

Just a UX note that the receipt fields shift left after the loading state, as in your screencast in the PR description.

jaclync · 2025-08-27T00:42:29Z

Modules/Sources/Yosemite/Tools/POS/PointOfSaleSettingsService.swift

+}
+
+public final class PointOfSaleSettingsService: PointOfSaleSettingsServiceProtocol {
+    public private(set) var siteID: Int64


nit: this could be let?

Suggested change

public private(set) var siteID: Int64

public let siteID: Int64

Adjusted 61728b2

jaclync · 2025-08-27T00:48:45Z

Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleSettingsServiceTests.swift

+    @Test func retrievePointOfSaleSettings_passes_correct_siteID_to_settingStoreMethods() async throws {
+        // Given
+        let differentSiteID: Int64 = 456
+        let customSUT = PointOfSaleSettingsService(siteID: differentSiteID,
+                                                   settingStoreMethods: settingStoreMethods)
+        settingStoreMethods.retrievePointOfSaleSettingsResult = .success([])
+
+        // When
+        _ = try await customSUT.retrievePointOfSaleSettings()
+
+        // Then
+        #expect(settingStoreMethods.retrievePointOfSaleSettingsCalled)
+        #expect(settingStoreMethods.retrievePointOfSaleSettingsSiteID == differentSiteID)
+    }


nit: this test case felt a bit duplicative, since the settings methods isn't initialized with a site ID and previous cases have tested on the site ID.

True, don't think is needed to check for siteID differences. Updated: 3b82a84

jaclync · 2025-08-27T01:24:50Z

Modules/Tests/YosemiteTests/PointOfSale/PointOfSaleSettingsServiceTests.swift

+        #expect(settingStoreMethods.retrievePointOfSaleSettingsSiteID == differentSiteID)
+    }
+
+    @Test func retrievePointOfSaleSettings_with_complete_pos_settings_then_returns_all_settings() async throws {


nit: could this be merged with retrievePointOfSaleSettings_when_successful_then_returns_expected_settings?

Yes! 2e5c028

jaclync · 2025-08-27T01:27:49Z

WooCommerce/Classes/POS/Models/PointOfSaleSettingsController.swift

+        if let defaultSiteName {
+            return defaultSiteName
+        } else {
+            return Localization.storeNotSet


super nit:

Suggested change

return Localization.storeNotSet

return Localization.storeNameNotSet

jaclync · 2025-08-27T01:32:52Z

WooCommerce/Classes/POS/Models/PointOfSaleSettingsController.swift

+    private let settingsService: PointOfSaleSettingsServiceProtocol
+    private let siteSettings: [SiteSetting]
+
+    init(settingsService: PointOfSaleSettingsServiceProtocol,


nit: maybe the site ID can be DI'ed to the controller as well? then siteID isn't needed in PointOfSaleSettingsServiceProtocol.

I think so 🤔 I've logged this one separately for now: WOOMOB-1188

jaclync · 2025-08-27T01:41:47Z

WooCommerce/Classes/POS/Presentation/Settings/PointOfSaleSettingsStoreDetailView.swift

+//#Preview {
+//    PointOfSaleSettingsStoreDetailView(posSettingsService: PointOfSaleSettingsController())
+//}


nit: let's remove commented out code

I forgot this one 🤦 . fixed: b7b3bee

jaclync · 2025-08-27T01:42:58Z

WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift

 import protocol Yosemite.PointOfSaleBarcodeScanServiceProtocol
 import enum Yosemite.PointOfSaleBarcodeScanError

+import class Yosemite.PointOfSaleSettingsService


super nit: is this import still needed?

Not needed! cf78712

jaclync · 2025-08-27T01:48:23Z

WooCommerce/Classes/POS/Models/PointOfSaleSettingsController.swift

+import class Yosemite.PointOfSaleSettingsService
+import Storage
+
+protocol PointOfSaleSettingsControllerProtocol {


nit: all the content of the protocol feels like specific to the store settings section PointOfSaleSettingsStoreDetailView, instead of the whole settings. For example, isLoading is just for the store section, but being in the settings controller makes it like it's the loading state for some global state in the settings.

Maybe we can keep the POS settings controller, as there could be other global states in the settings later, and a separate @Observable view model like class to provide the following interface for PointOfSaleSettingsStoreDetailView?

Good point, I've added a note on WOOMOB-1188 so signature and dependencies can be adjusted in one go 👍

jaclync · 2025-08-27T01:49:30Z

WooCommerce/WooCommerceTests/POS/Presentation/POSItemActionHandlerTests.swift

    couponsSearchController: PointOfSaleSearchingItemsControllerProtocol = MockPointOfSaleCouponsController(),
    cardPresentPaymentService: CardPresentPaymentFacade = MockCardPresentPaymentService(),
    orderController: PointOfSaleOrderControllerProtocol = MockPointOfSaleOrderController(),
+    settingsController: PointOfSaleSettingsControllerProtocol = PointOfSaleSettingsPreviewController(),


nit: we may want a mock version in unit tests.

Addressed here d036798

jaclync · 2025-08-27T01:52:07Z

WooCommerce/Classes/POS/Presentation/PointOfSaleDashboardView.swift

@@ -1,4 +1,5 @@
 import SwiftUI
+import class Yosemite.PointOfSaleSettingsService


super nit: is this import still needed?

Not needed! cf78712

# Conflicts: # WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift # WooCommerce/Classes/POS/Presentation/Settings/PointOfSaleSettingsView.swift

iamgabrielma added 7 commits August 25, 2025 14:38

make PointOfSaleSettingsService

059bcd9

update init

433a5d8

move version check to service

4713200

make settingValueView

af3b76e

early return if version not supported

4201416

cleanup

7b66d8f

update NSLocalizedStrings

46035e4

iamgabrielma added 5 commits August 25, 2025 16:03

lint and POS fonts

e49dad7

move setting retrieval to the parent view

4bb9542

update imports and localization

03834df

make setting view create its own service

8a02151

Revert "make setting view create its own service"

fe32ce4

This reverts commit 8a02151.

iamgabrielma added type: task An internally driven task. feature: POS labels Aug 25, 2025

iamgabrielma added this to the 23.2 milestone Aug 25, 2025

iamgabrielma marked this pull request as ready for review August 25, 2025 10:27

iamgabrielma requested a review from jaclync August 25, 2025 10:46

jaclync reviewed Aug 26, 2025

View reviewed changes

iamgabrielma added 3 commits August 26, 2025 13:17

split settings service into controller-service

e19018b

extract controller init to posmodel

c3c2ca0

make protocol. DI settings controller into POS aggregate

8f91b68

iamgabrielma added 6 commits August 26, 2025 15:30

make test target compile

a626d12

DI storage into settings service

b102193

DI default values for store name and address

f3de3be

add PointOfSaleSettingsServiceTests

9bedfa1

make PointOfSaleSettingsServiceProtocol and add tests

50c3efe

clean up unused storage

e2e84b2

iamgabrielma commented Aug 26, 2025

View reviewed changes

WooCommerce/Classes/POS/Models/PointOfSaleSettingsController.swift Show resolved Hide resolved

restore commented out preview

72bed3b

iamgabrielma commented Aug 26, 2025

View reviewed changes

WooCommerce/Classes/POS/Models/PointOfSaleSettingsController.swift Show resolved Hide resolved

iamgabrielma requested a review from jaclync August 26, 2025 11:09

Base automatically changed from task/WOOMOB-1025-two-panel-settings-view to trunk August 27, 2025 00:38

jaclync approved these changes Aug 27, 2025

View reviewed changes

iamgabrielma added 8 commits August 27, 2025 10:09

Merge branch 'trunk' into task/WOOMOB-1040-POSSettings-store-section

ae533c5

# Conflicts: # WooCommerce/Classes/POS/Models/PointOfSaleAggregateModel.swift # WooCommerce/Classes/POS/Presentation/Settings/PointOfSaleSettingsView.swift

remove unnecessary test

3b82a84

adjust access control

61728b2

update var name

0398758

merge tests

2e5c028

make mock and wrap preview in debug flag

d036798

update preview

b7b3bee

remove unnecessary imports

cf78712

iamgabrielma enabled auto-merge August 27, 2025 03:49

iamgabrielma merged commit 722d258 into trunk Aug 27, 2025
14 checks passed

iamgabrielma deleted the task/WOOMOB-1040-POSSettings-store-section branch August 27, 2025 03:58

iamgabrielma mentioned this pull request Aug 28, 2025

[POS Settings] Store settings section #16032

Closed

1 task

	public private(set) var siteID: Int64
	public let siteID: Int64

	return Localization.storeNotSet
	return Localization.storeNameNotSet

		@@ -1,4 +1,5 @@
		import SwiftUI
		import class Yosemite.PointOfSaleSettingsService

[POS Settings] Store section and PointOfSaleSettingsService #16034

[POS Settings] Store section and PointOfSaleSettingsService #16034

Uh oh!

Conversation

iamgabrielma commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing information

Question/Feedback

Uh oh!

wpmobilebot commented Aug 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaclync left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dangermattic commented Aug 26, 2025

Uh oh!

Uh oh!

Uh oh!

iamgabrielma commented Aug 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jaclync left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

[POS Settings] Store section and `PointOfSaleSettingsService` #16034

[POS Settings] Store section and `PointOfSaleSettingsService` #16034

iamgabrielma commented Aug 25, 2025 •

edited

Loading

wpmobilebot commented Aug 25, 2025 •

edited

Loading

jaclync left a comment •

edited

Loading

iamgabrielma commented Aug 26, 2025 •

edited

Loading